Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(common): locales are not being shipped #23136

Closed
wants to merge 1 commit into from
Closed

fix(common): locales are not being shipped #23136

wants to merge 1 commit into from

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Apr 3, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #23140, #23103

What is the new behavior?

locales are shipped

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

@alan-agius4 alan-agius4 changed the title fix: common fix(common): locales are not being shipped Apr 3, 2018
@alan-agius4
Copy link
Contributor Author

@alexeagle can you have a quick look please? Thanks :)

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 3, 2018

yarn buildifier is throwing an error on windows even in admin mode

λ yarn buildifier
yarn run v1.3.2
$ bazel build --noshow_progress @com_github_bazelbuild_buildtools//buildifier
DEBUG: C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/io_bazel_rules_go/go/private/toolchain.bzl:104:5: mkdir C:\WINDOWS\go-build100047967: Access is denied.
ERROR: C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/io_bazel_rules_go/go/toolchain/BUILD.bazel:6:1: every rule of type _go_toolchain implicitly depends upon the target '@go_sdk//:packages.txt', but this target could not be found because of: no such package '@go_sdk//': Traceback (most recent call last):
        File "C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/io_bazel_rules_go/go/private/toolchain.bzl", line 45
                _prepare(ctx)
        File "C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/io_bazel_rules_go/go/private/toolchain.bzl", line 105, in _prepare
                fail("failed to list standard package...")
failed to list standard packages
ERROR: Analysis of target '@com_github_bazelbuild_buildtools//buildifier:buildifier' failed; build aborted: no such package '@go_sdk//': Traceback (most recent call last):
        File "C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/io_bazel_rules_go/go/private/toolchain.bzl", line 45
                _prepare(ctx)
        File "C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/io_bazel_rules_go/go/private/toolchain.bzl", line 105, in _prepare
                fail("failed to list standard package...")
failed to list standard packages
INFO: Elapsed time: 29.225s
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

After I ran bazel clean --expunge

I will get

INFO: Analysed target @com_github_bazelbuild_buildtools//buildifier:buildifier (23 packages loaded).
INFO: Found 1 target...
ERROR: C:/users/alag/appdata/local/temp/_bazel_alag/btbwur4a/external/go_stdlib_windows_amd64_cgo/BUILD.bazel:4:1: error executing shell command: 'export GOROOT=$(pwd)/bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC=$(pwd)/C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/amd64/cl.ex...' failed (Exit 1)
/usr/bin/bash: -c: line 0: syntax error near unexpected token `('
/usr/bin/bash: -c: line 0: `export GOROOT=$(pwd)/bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC=$(pwd)/C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/amd64/cl.exe CXX=$(pwd)/C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/amd64/cl.exe COMPILER_PATH=C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/amd64 && mkdir -p bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/src && mkdir -p bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/pkg && cp external/go_sdk//bin/go bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/bin/go && cp -rf external/go_sdk//src/* bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/src/ && cp -rf external/go_sdk//pkg/tool bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/pkg/ && cp -rf external/go_sdk//pkg/include bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/pkg/ && bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/bin/go install  std && bazel-out/host/bin/external/go_stdlib_windows_amd64_cgo/bin/go install  runtime/cgo'
Target @com_github_bazelbuild_buildtools//buildifier:buildifier failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 101.472s, Critical Path: 2.98s
error Command failed with exit code 1.

@alan-agius4
Copy link
Contributor Author

fixed the lint on a mac

@alexeagle
Copy link
Contributor

Let's file the issue with rules_go - I don't want to leave Windows broken...

Could you try upgrading the Go rules first - change /WORKSPACE to this version
https://github.com/alexeagle/angular-bazel-example/blob/master/WORKSPACE#L65

So that our issue report is against their current release (or maybe it will be solved by updating)

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 3, 2018 via email

Copy link
Contributor

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
It's simpler than I expected - I thought these nested locales directories needed to appear as entry points. But checking the output from the old build.sh (still published as https://github.com/angular/common-builds) this seems right to me.

@alexeagle alexeagle added target: major This PR is targeted for the next major release action: merge The PR is ready for merge by the caretaker labels Apr 3, 2018
@alxhub alxhub closed this in 7ca7720 Apr 3, 2018
@alan-agius4 alan-agius4 deleted the feature/common-locales branch April 3, 2018 18:08
@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 3, 2018

Hi @alexeagle & @IgorMinar I relized I did a small mistake I did a nested it in the tests which was causing the tests to give false positives. I did a PR to change this. However, it s failing because in the data arg only .d.ts files are being passed. I check some other implementations of the data attr, ex: https://github.com/angular/angular/blob/master/packages/compiler/test/BUILD.bazel#L66 and cannot really quite get why only .d.ts files are passed. I can see that the js and not map files are being generated as they are being now inlined (not sure if this is by design, I do see "inlineSources": true, in tsconfig-build). if I do cd ../locales from bin/packages/common/npm_package.

Any pointers where should I look?

PR: #23153

I am really sorry for this guys. But I didn't notice earlier about this mistake and problem, and the tests where not giving an error.

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 4, 2018

Hey @alexeagle.

As promised I have tried to update the Go Rules on Windows. Now the error message have changed to something else. It seems the error is more related to Bazel and is already tracked here: bazelbuild/bazel#4496

Do you want me to push the go rules update anyways since it’s a step forward?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants